breaking: the effect-stack lifecycle (#58): transports own their link, options, and sizing - #144

fix(udp): cap the payload at the server's advertised buffer

…request

A verbatim move with no behavior change: the TypeVars, the `success`/`error`
narrowers, `wrapping_sequence`, the validation diagnostics, and the body of
`SMPClient.request`, which becomes `_request.exchange(transport, request,
sequence, timeout_s)`. `SMPClient.request` delegates to it, and `smpclient`
re-exports every public name unchanged.

The next commit needs this: a transport reads the server's MCUmgr parameters
while it connects, before any `SMPClient` exists, so the exchange must sit below
the client. Review with `git show --color-moved`; 252 of the 284 changed lines
are moved.

`smpclient` imports `_request` as a module (`from smpclient import _request`),
so the package attribute stays the submodule. An `import ... as _request` of the
function would shadow it and break `mock.patch("smpclient._request.…")`. The
function is named `exchange` so that its `request` parameter doesn't shadow it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…lient never opens a link

`SMPClient(transport, address, timeout_s)` becomes `SMPClient(transport, *, timeout_s,
sequence)`. `connect()`, `disconnect()`, `address`, `__aenter__`/`__aexit__` and
`_initialize()` are removed. The client only sends and receives over a transport that
is already open. It holds the `SMPTransport` Protocol, which no longer has `connect` or
`disconnect`, so it can't control the transport's side effects.

Each transport now takes its address in the constructor (#58), along with
`connect_timeout_s` and `sequence`. `connect()` and `disconnect()` take no arguments,
and their bodies are unchanged:

- serial: the old `connect` body becomes `_open()`; `connect()` is `_open()` then
  `negotiate()`
- UDP: `SMPUDPTransport(address, port=1337, *, mtu, ...)`. The port is a real
  parameter, the point of #58.
- bleak: `connect()` wraps `_connect(address, timeout_s)`, which is unchanged
- bumble: `connect()` is the old flow, reading the address from `self`.
  `use_connection` becomes `borrow`, and the module helper `borrowed_connection()`
  becomes the method `borrowed()`.

A private base, `_ConnectableTransport`, adds the encouraged bracket,
`async with transport.connected():`. It connects, yields the transport, and
disconnects best-effort. The primitives remain for lifetimes a lexical scope can't
express, e.g. a standing link held for an application's lifetime.

The MCUmgr parameters read moves out of `SMPClient._initialize` into the
transport's `negotiate()`, with the same warnings and the same fallback on an error
or a timeout (`_request.read_mcumgr_parameters`). `negotiate()` runs inside
`connect()` and `borrow()`, and it is public, for re-negotiating: the integration
harness uses it after a server boots, and a borrowed link can negotiate at all.
Behavior is unchanged here: the read is still unconditional. The next commit makes
it conditional on each transport's fragmentation strategy.

`connect()` is all-or-nothing on every transport: a failed or cancelled negotiation
closes the link it just opened.

Tests move to address-first constructors and argument-free `connect()`. A
`skip_negotiation` fixture answers the params read with `None`, so tests that drive
`connect()` over mocked I/O don't wait for a server. The integration harness enters
`transport.connected()` and re-negotiates after the echo wait, where it used to call
`client._initialize()`. Its skip for a UDP fixture on a non-default port is gone.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…e only when asked

Each transport now declares how it sizes SMP messages with its own
fragmentation strategy union, and reads the server's MCUmgr parameters
only when that strategy asks for them. A pinned strategy never issues
the read, so a server without the params command, like mcuboot serial
recovery, never sees it.

The shared vocabulary lives in `smpclient.transport`:

- `Auto`: read `buf_size` while connecting. On a timeout or an error
  response, warn and fall back to the transport's conservative default.
- `BufferSize(buf_size)`: a known server buffer; nothing is read.
- `Unfragmented`: GATT only. Like `Auto`, but one message per write,
  for a server built without `CONFIG_MCUMGR_TRANSPORT_BT_REASSEMBLY`.

The per-transport unions use the prefixed names:

- `SerialFragmentationStrategy = Auto | BufferSize | BufferParams`
  (serial's own `BufferSize(buf_size, line_length)` and `BufferParams`
  are unchanged from main)
- `RawSerialFragmentationStrategy = Auto | BufferSize`
- `UDPFragmentationStrategy = Auto | BufferSize`, always capped at the
  MSS
- `GATTFragmentationStrategy = Auto | Unfragmented | BufferSize`, shared
  by `SMPBLETransport` and `SMPBumbleTransport` through a `_GATTTransport`
  mixin

`SMPTransport.initialize()` and `_smp_server_transport_buffer_size` are
gone. Each transport's `negotiate()` matches its strategy exhaustively
and stores `_negotiated_buf_size`; `max_unencoded_size` is derived from
the strategy.

Breaking:

- `smpclient.transport.serial.FragmentationStrategy` is renamed
  `SerialFragmentationStrategy`.
- `Auto` moves to `smpclient.transport`, since every transport uses it.
- `SMPSerialRawTransport(port, mtu=384)` becomes
  `SMPSerialRawTransport(port, fragmentation_strategy=Auto())`; pin the
  old behavior with `BufferSize(384)`. Its `mtu` now reports
  `max_unencoded_size`, one whole message.
- The serial "pinned size exceeds the server's buffer" warnings are
  removed: a pinned strategy no longer reads the parameters it would
  compare against.

Tests: `tests/support.py` adds `advertise(buf_size)`, which patches the
params read, and `negotiated(transport, buf_size)`. Each transport tests
that a pinned strategy never reads, and how `Auto` and `Unfragmented` cap
the size. The integration raw transport defaults to `Auto()`, so the
suite exercises negotiation against the real fixtures (229 passed, 101
skipped, the same as before).

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

The eleven pyserial keyword arguments are replaced by one
`options: SerialOptions = SerialOptions()` on `SMPSerialTransport`
(all three constructor overloads and the implementation) and on
`SMPSerialRawTransport`. The settings are declared once, in
`smpclient.transport.serial.common`, and exported from
`smpclient.transport.serial`; they are no longer repeated across the
four encoded signatures, the raw signature, and the base.

    SMPSerialTransport(port, baudrate=9600)
    SMPSerialTransport(port, options=SerialOptions(baudrate=9600))

`test_serial_options_lock_pyserial` locks the field names, their order,
and their defaults to `inspect.signature(serial.Serial)`. The one
deliberate difference is `baudrate`: 115200 here, 9600 in pyserial.
pyserial is effectively unmaintained, so drift isn't expected, but the
test fails loudly if it happens. A renamed field and a changed default
were each confirmed to fail the test.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… flush fails

`_open()` now runs pyserial's blocking `open()` and
`reset_input_buffer()` in `asyncio.to_thread`, so a slow open (USB CDC
ACM still enumerating, for example) no longer stalls the event loop.

The leak, which is pre-existing on main: `reset_input_buffer()` was
inside the retry `try`. When the flush raised `SerialException` after
a successful `open()`, the loop called `open()` again on the open port.
pyserial refuses that with another `SerialException`, so the loop spun
until `connect_timeout_s` and raised `TimeoutError`, leaving the first
fd open. Only `open()` is retried now (`try/except/else`), and
`connect()` wraps `_open()` too in its all-or-nothing
`except (Exception, CancelledError): close()`. A failed flush, or a
cancellation during the open, closes the port and re-raises.

A cancellation that lands while the worker thread is still inside
`open()` cannot interrupt that thread. The best-effort `close()` runs
either way, and is a no-op if the open hadn't finished.

Tests: `test_connect_closes_the_port_when_the_flush_fails` fails on the
previous code. `test_connect_closes_the_port_when_cancelled_while_negotiating`
covers the cancel path.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…cket chardev

`_SerialTransportBase` gains the same borrow primitives the GATT
transports have:

- `await t.borrow(port)` adopts a caller's open `SerialPort`, clears the
  framing state, then runs `negotiate()`. It is all-or-nothing: if
  negotiation raises or is cancelled, the transport reverts to its own
  port.
- `async with t.borrowed(port):` wraps borrow and disconnect in a
  bracket.
- `disconnect()` returns a borrowed port without closing it; the
  acquirer releases. It still closes the transport's own `Serial`.

`SerialPort` is the Protocol for the four members the transports use:
`port`, `out_waiting`, `write`, and `read_all`. `serial.Serial`
satisfies it, and so does a `serial_for_url` port that reports
`out_waiting`. Both it and `SerialOptions` are exported from
`smpclient.transport.serial`.

Which port is live is a sum type, `_Link = _Owned | _Borrowed(port)`.
`_conn` becomes a property that matches on it, and `_serial` is the
transport's own `Serial`, still constructed closed in `__init__`. The
unit tests' `t._conn.<attr> = MagicMock(...)` assignments still land on
the owned mock, so they keep working unchanged.

Integration harness: the `QemuSocketSerialTransport` and
`QemuSocketSerialRawTransport` subclasses are gone. They overrode `_open`
and replaced the `Final` `_conn` with `object.__setattr__`. Now
`socket_link(transport, url)` opens the emulator's `socket://` chardev
(paced for the raw transport), lends it with `transport.borrowed()`,
and closes it on exit, so the suite drives the real public API.
`ConnectedServer` carries its link as an `AsyncExitStack`, and
`reboot_into_recovery` releases it with `link.aclose()` before opening
the recovery link it is handed. Integration: 229 passed, 101 skipped,
the same as before, with every socket fixture going through `borrowed()`.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

`SMPBLETransport` gains the borrow primitives the other transports
have:

- `await t.borrow(client)` adopts a caller's connected `BleakClient`,
  finds the SMP characteristic, sizes writes to the link, subscribes,
  and then runs `negotiate()`. It is all-or-nothing: on failure or
  cancellation the transport returns the client and re-raises.
- `async with t.borrowed(client):` wraps borrow and disconnect in a
  bracket.
- `disconnect()` on a borrowed client unsubscribes and never
  disconnects it; the acquirer releases. The `stop_notify` is bounded
  by `connect_timeout_s`, and a failure is logged rather than raised, so
  returning the client can't hang or mask the caller's error when the
  owner has already dropped the link.

The part of `_connect()` after the link comes up is now `_start_smp()`,
shared by connect and borrow, and it clears the receive buffer.
Ownership is a sum type, `_Link = _Owned | _Borrowed(client)`.
`_active_client` matches on it; `_client` stays the transport's own
client, so the unit tests that assign it keep working.

Disconnect detection: bleak takes `disconnected_callback` only when
the client is constructed, and the owner holds it. So
`_until_disconnected()` waits on the transport's event when it owns the
client, and polls `client.is_connected` every 100 ms when it borrows
one. The poll runs only inside a receive or GATT wait. There is no
watcher task, so nothing outlives the primitive that started it.

`_notify_or_disconnect` now reaps its two sub-tasks in a `finally`, like
`_await_or_disconnect`. Before, cancelling a waiting `receive()` leaked
both tasks; with a borrowed client, that would leave a poll loop
running for as long as the owner's link stayed up. The old
`except CancelledError: pass` around the reaping `gather` also
swallowed a cancellation of the waiter itself; a positional
`gather(..., return_exceptions=True)` doesn't.
`test_borrowed_receive_leaves_no_task_polling_when_cancelled` fails
without the `finally`.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

`SMPBLETransport(address, bluez=BlueZClientArgs(adapter="hci1"))`
scans for the device and connects to it on that adapter, instead of
bleak's default. The options are bleak's own type, so the
`BlueZClientArgs` passes to `BleakClient`, and its keys, a subset of
`BlueZScannerArgs`, pass to `find_device_by_address` and
`find_device_by_name`. `SMPBLETransport.scan()` takes
`bluez: BlueZScannerArgs` too. Both default to `{}`, which is bleak's
default adapter, so behavior is unchanged.

This sits beside the existing `winrt=WinRTClientArgs(...)`, which
already carries #90's `use_cached_services`. bleak is pinned `>=3.0.2`,
and the BlueZ adapter args date from 3.0.

Closes #103

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

`SMPBumbleTransport.bonded_devices()`, `.clear_bond(address)`, and
`.clear_bonds()` become module functions in `smpclient.transport.bumble`:

    await bonded_devices(keystore=..., host_address=...)
    await clear_bond(address, keystore=..., host_address=...)
    await clear_bonds(keystore=..., host_address=...)

They only ever read the transport's `keystore` and `host_address`, the
keystore namespace, and never its link. So listing or clearing bonds
no longer means building a transport for a device address you don't
plan to connect to. The defaults match the transport's, `Tempfile()`
and `DEFAULT_HOST_ADDRESS`, so a call with none of the options sees the
same bonds a default transport writes. The private
`_standalone_keystore()` helper is gone.

The functions had no tests; `test_bond_functions_manage_the_hosts_bonds`
seeds a keystore, then covers list, per-host isolation, clearing one
bond, and clearing all.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…ibe only ourselves

Three lifecycle gaps from the #144 review:

- A cancelled `connect()` leaked its partial state. The teardown arm was
  `except Exception`, which `CancelledError` bypasses, so a cancel mid
  `device.connect()` left the state at `Connecting` with the HCI
  transport open. Every later `connect()` then raised "called while in
  state Connecting". A `CancelledError` arm now tears down and
  re-raises, logged at debug: a cancel is the caller's decision, not an
  error. This is pre-existing on main.
- `borrow()` was not all-or-nothing. If `negotiate()` raised or was
  cancelled, the transport stayed `ConnectedBorrowed`, subscribed, with
  its disconnection listener attached. It now returns the connection
  (`disconnect()` → `_teardown_borrowed`) and re-raises, like serial and
  bleak `borrow()`.
- Returning a borrowed connection called `smp_characteristic.unsubscribe()`
  with no subscriber. bumble reads that as "drop every subscriber" and
  writes the CCCD to zero, cutting off the owner's own notifications on
  the shared characteristic. It now passes `self._on_notification`.
  bumble keys subscriber proxies by the subscriber, and a bound method
  compares equal each time it's looked up, so only this transport's
  proxy is removed, and the CCCD is cleared only if no subscriber is
  left. The owned teardown still unsubscribes everything; it owns the
  whole link.

Each new test fails on the previous code.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

…ises SMPTransportDisconnected

`_Link` is now `_Closed | _Owned(client) | _Borrowed(client)`, and the
separate `_client` attribute is gone. `connect()` creates
`_Owned(BleakClient(...))`; `borrow()` makes `_Borrowed(client)`; and
`disconnect()` ends in `_Closed()` from any state, so it stays
idempotent.

This closes a gap that was also on `main`. Once a transport that only
ever borrowed had returned its client, `disconnect()` switched back to
the old `_Owned()` marker, and `_active_client` read a `self._client`
that only `connect()` assigns. `send()`/`receive()` raised
`AttributeError: _client` instead of `SMPTransportDisconnected`; the
same was true on `main` for a transport that never connected. With the
client inside the variant, "no client" is its own case:

- `_active_client` raises `SMPTransportDisconnected` on `_Closed`.
- `_until_disconnected` returns at once on `_Closed`.
- `_best_effort_disconnect` delegates to `disconnect()`, dropping its
  defensive `getattr`.
- `_set_disconnected_event` still rejects a callback from a client
  other than the owned one. After our own `disconnect()` the link is
  `_Closed`, so bleak's callback for that disconnect is accepted.

Tests inject `t._link = _Owned(client)`, or read the owned client with
`_owned_client(t)`, instead of assigning `t._client`. Four sizing tests
dropped a client assignment they never used.
`test_a_returned_borrow_raises_disconnected` fails on the previous code
with the `AttributeError`, and `test_disconnect` now also checks
idempotence and a send after close.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… no Optional sizing state

Per review on #144 ("Are these final?", "you say it defaults to
wrapping_sequence, yet you default it to None in the sig", "less
mutation"):

- `_ConnectableTransport` is now a concrete base, generic over the
  transport's fragmentation strategy union. Its `__init__` sets
  `_fragmentation_strategy`, `_connect_timeout_s`, and `_sequence` as
  `Final`. As Protocol members they could not be `Final`: a `Final`
  assignment in an implementer conflicts with a writable Protocol
  attribute. `connect`/`disconnect`/`negotiate` are `@abstractmethod`,
  and their docstrings point at the `connected()` bracket.
- `sequence: Iterator[u8] | None = None` becomes
  `sequence: Callable[[], Iterator[u8]] = wrapping_sequence` on every
  transport and on `SMPClient`, so the signature states the default.
  A factory rather than an iterator, because a default iterator is
  evaluated once at definition time and would be shared across
  instances. The "defaults to `wrapping_sequence()`" prose is gone.
- `_negotiated_buf_size: int | None` is gone. The configured strategy
  stays `Final`, and one slot, `_sizing`, holds the strategy as
  `negotiate()` resolved it. `Auto` resolves to `BufferSize(n)` when the
  server advertises `n`; GATT `Unfragmented` resolves to
  `BufferSize(min(mtu, n))`. An unadvertised read resets to the
  unresolved strategy, so a re-negotiation never keeps a stale size from
  an earlier server. Every sizing property now matches `_sizing` with
  no `is None` branches; `Auto` there only means "the server advertised
  nothing".
- Attributes that are never reassigned are `Final`: UDP's `_mtu`,
  bleak's `_buffer`/`_notify_condition`/`_disconnected_event`/`_winrt`,
  and `SMPClient._timeout_s`.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… the constructor

Per review on #144 ("Port wouldn't be required if we achieve via
SMPSerialTransport.borrowed()... right?"): a transport that only
borrows never needed a port or address. The constructor now holds only
config: the fragmentation strategy, options, timeouts, and sequence.
The target belongs to the primitive that opens a link.

    async with SMPSerialTransport(BufferSize(1024)).connected("/dev/ttyACM0") as t: ...
    async with SMPSerialTransport().borrowed(open_port) as t: ...
    async with SMPUDPTransport().connected("192.168.1.1", 1337) as t: ...
    async with SMPBLETransport().connected("AA:BB:CC:DD:EE:FF") as t: ...
    async with SMPBumbleTransport(hci="usb:0").connected("AA:BB:CC:DD:EE:FF") as t: ...

- `connect(target)` and `connected(target)` are defined per transport:
  serial `port`, BLE/bumble `address`, UDP `address, port=1337`. The
  signatures differ, so `_ConnectableTransport` no longer declares
  `connect`. What each bracket shares is `_released_on_exit()`, which
  yields the link and releases it best-effort on every exit. Both
  `connected()` and `borrowed()` use it.
- The primitives' docstrings point at their bracket, and the base
  class docstring says what the bare primitives give up.
- The positional constructor arguments are now the ones `main` had:
  serial and raw serial take `fragmentation_strategy`; UDP takes `mtu`.
- Tests, the integration harness (`_link`, `socket_link`, the recovery
  and line-length tests), the examples, the `SMPClient` docstring
  example, and the bumble CLI all pass the target to
  `connect`/`connected`.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… at once

Per review on #144 ("You're telling me that we accept both? On macos
too? At the same time... This is a sum type."): `winrt=` and `bluez=`
could both be passed, and neither means anything on macOS. They become
one `backend` argument:

    BleakBackend = PlatformDefault | BlueZ | WinRT

    SMPBLETransport(backend=BlueZ(BlueZClientArgs(adapter="hci1")))
    SMPBLETransport(backend=WinRT(WinRTClientArgs(use_cached_services=True)))
    SMPBLETransport()  # PlatformDefault()

The variants wrap bleak's own `BlueZClientArgs`/`WinRTClientArgs`, so
bleak's option schema stays the single source of truth. `_bluez_args`
and `_winrt_args` each match the backend exhaustively into the
`bluez=`/`winrt=` that `BleakClient` and `BleakScanner` take; the
backend that wasn't chosen gets `{}`. `SMPBLETransport.scan()` takes the
same `backend` instead of `bluez: BlueZScannerArgs = {}`, which also
drops a mutable default argument.

`test_connect_passes_bleak_only_the_chosen_backend` checks, for each
variant, what the scanner and the client receive.

Closes #103

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Per review on #144 ("Should be generic over the transport, so that
users can access the transport in a type safe way."):

    TTransport = TypeVar("TTransport", bound=SMPTransport)
    class SMPClient(Generic[TTransport]): ...
        @Property
        def transport(self) -> TTransport: ...

`SMPClient(SMPSerialTransport())` is an `SMPClient[SMPSerialTransport]`,
so `client.transport.read_serial()` type-checks with no cast.
`ICUploadClient` is generic over the same `TTransport`.
`SMPClient.__init__` gains its missing `-> None`.

`tests/test_generics_typing.py` asserts the type of `client.transport`
for both classes under mypy and pyright. Typing the property as plain
`SMPTransport` fails that check.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

… positional bond args

Per review on #144 ("llm doc slop - restates the code and other doc
strings", "why kwargs only?"):

- The strategy aliases no longer list their own members or restate the
  sizing rules the properties implement. `SerialFragmentationStrategy`,
  `RawSerialFragmentationStrategy`, and `UDPFragmentationStrategy` are
  each one line. The constructors' `fragmentation_strategy` Args no
  longer repeat the union.
- `_request.exchange` and `_request.read_mcumgr_parameters` are private
  helpers, so their docstrings are one line. `exchange` was a copy of
  `SMPClient.request`'s docstring, which remains the documented
  contract.
- The nested `match` on the params read (`int | None`) ended in a bare
  capture, which rules out an `assert_never` arm. Each now matches
  `case int() as buf_size:` and closes with `case _ as unreachable:
  assert_never(unreachable)`. That covers all five transports'
  `negotiate()`, including encoded serial's guarded arm.
- `bonded_devices`, `clear_bond`, and `clear_bonds` drop the `*`. There
  is no ambiguity for keyword-only to guard against.
- Test comments no longer mention the removed 7.1.0 params.
- `serial.common` reuses `smpclient.transport._TStrategy` instead of
  redeclaring an identical TypeVar.

Refs #58

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>